Keep QuoteContext found in typer for quoted.Type #8949
Conversation
nicolasstucki
force-pushed
the
find-quoted-type-context-early
branch
6 times, most recently
from
3b6f80c to
6746d17
Compare
| // '[ x.$splice ] --> x | ||
| transform(t) | ||
| case _ => | ||
| super.transform(tree) |
There was a problem hiding this comment.
Does the branch catch the case where the splice is in a big type, such as '{ e: F[$x]}?
| @@ -1,3 +1,3 @@ | |||
| object Test { | |||
| def test(using quoted.QuoteContext) = { | |||
| Macro.ff(3) | |||
| } | |||
There was a problem hiding this comment.
It's not clear why using quoted.QuoteContext is necessary for macro usage.
There was a problem hiding this comment.
The test is awkward. The inline method is the one that requires the quoted.Type as an implicit. This might only be used to write a macro that is used in a macro implementation, hence the QuoteContext.
Previously it worked because we never used the Type and by chance it stayed encoded as a QuoteContext ?=> Type[T] which was never called. That is why we missed the need for a QuoteContext.
The current approach homogeneously requires the quote context for all Type quotes as we do with Expr quotes. This helps to find out missing QuoteContext early in the pipeline.
tests/pos-macros/i8302.scala
Outdated
| @@ -1,6 +1,6 @@ | |||
| import scala.quoted._ | |||
| def foo[T](using qctx: QuoteContext, tpe: Type[T]): Expr[Any] = | |||
| '{ | |||
| '{ (qctx: QuoteContext) ?=> | |||
There was a problem hiding this comment.
Is this necessary? BTW, the recommended syntax is (using QuoteContext) => e if I remember correctly.
There was a problem hiding this comment.
Yes, that is the quote context for the next stage. Similar reason as the previous one. Will change the syntax.
| @@ -1,30 +1,30 @@ | |||
| import scala.quoted._ | |||
|
|
|||
|
|
|||
| inline def isFunctionType[T:Type]: Boolean = ${ isFunctionTypeImpl('[T]) } | |||
| inline def isFunctionType[T]: Boolean = ${ isFunctionTypeImpl('[T]) } | |||
There was a problem hiding this comment.
This is nice, improves meta-programming experience a lot 👍
There was a problem hiding this comment.
This proves the point made in the previous comment. Here the calls failed to compile and I noticed at once that we had an extra Type that was not needed.
| @@ -92,6 +92,13 @@ abstract class TreeMapWithStages(@constructorOnly ictx: Context) extends TreeMap | |||
| } | |||
|
|
|||
| tree match { | |||
| case Apply(Select(Quoted(quotedTree), _), _) if quotedTree.isType => | |||
There was a problem hiding this comment.
The pattern does not seem to have enough guards, could it match unintended trees by accident?
There was a problem hiding this comment.
The guard are inside the Quoted extractor. Only the compiler generates these trees and the rest of the shape is know. I will do a refactor of the Quoted/Spliced extractor at some point, but I would prefer to do it on it's own PR.
We already do this for quoted terms. We missed this last one. After this change we won't need to do implicit search after pickling anymore.
nicolasstucki
force-pushed
the
find-quoted-type-context-early
branch
from
6746d17 to
47a1586
Compare
Co-authored-by: Fengyun Liu <liu@fengy.me>
We already do this for quoted terms. We missed this last one.
After this change, we won't need to do implicit search after pickling anymore.
QuoteContextwhen synthesizingType(instead of inReifyQuotes)givenfromType. XYZTag(already handled by synthesized types and quote reification)Typecontext bounds from tests (never needed)